Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added pyproject extra to install toml package as an optional dependency #964

Merged
merged 2 commits into from
Feb 8, 2022
Merged

Added pyproject extra to install toml package as an optional dependency #964

merged 2 commits into from
Feb 8, 2022
+4 −0

Conversation

arareko
Copy link
Contributor

@arareko arareko commented Sep 20, 2021

This patch fixes #936

@google-cla google-cla bot added the cla: yes label Sep 20, 2021
@GrahamCampbell
Copy link

👎 Should be marked as an optional dependency.

@arareko
Copy link
Contributor Author

arareko commented Sep 24, 2021

👎 Should be marked as an optional dependency.

@GrahamCampbell not optional when it is being used directly in one of the YAPF modules.

@GrahamCampbell
Copy link

The code expects it to be missing and handles that case. Totally not the same as just straight up importing it.

@bwendling
Copy link
Member

I have to agree with @GrahamCampbell here.

@ghost
Copy link

ghost commented Sep 25, 2021

I think this change is reasonable considering pyproject.toml is becoming standard for declaring build systems, but also what mentioned in #936.. or at least I do find it unusual to have to add toml as a dep myself when using poetry with yapf

@arareko
Copy link
Contributor Author

arareko commented Oct 7, 2021

@gwelymernans I have made the necessary adjustments.

@arareko arareko changed the title Add toml package as a dependency Added pyproject extra to install toml package as an optional dependency Oct 7, 2021
@pedro757
Copy link

pedro757 commented Jan 24, 2022 •
edited
Loading

Question: why toml can not simply be a normal dependency?

I mean, in the end yapf is just a developer dependency it actually wont affect any program performance.

@pfernandesviz
Copy link

Can you guys merge this?

@bwendling
Copy link
Member

Sorry about the lateness. Done.

@bwendling bwendling merged commit fb0fbb4 into google:main Feb 8, 2022
@arareko arareko deleted the add-toml-as-dependency branch February 8, 2022 23:55
@casperdcl casperdcl mentioned this pull request Mar 23, 2022
Open
gadomski added a commit to stac-utils/stactools that referenced this pull request Mar 25, 2022
@gadomski
deps: add pyproject extra_requires to yapf
5fe930a 
google/yapf#964
gadomski added a commit to stac-utils/stactools that referenced this pull request Mar 25, 2022
@gadomski
deps: add toml to requirements-dev
64424d8 
google/yapf#964 needs to be released before we can use
`yapf[pyproject]`.
gadomski added a commit to stac-utils/stactools that referenced this pull request Mar 28, 2022
@gadomski
deps: add toml to requirements-dev
a53bfdc 
google/yapf#964 needs to be released before we can use
`yapf[pyproject]`.
@wrench1815 wrench1815 mentioned this pull request Oct 27, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

toml is missing as a dependency
5 participants
@arareko @GrahamCampbell @bwendling @pedro757 @pfernandesviz